[chain] Apply exported lint rule#2018
Conversation
| exclude: | ||
| - "**/abi/**/*.go" | ||
| - "**/api/**/*.go" | ||
| - "**/auth/**/*.go" | ||
| - "**/chainindexer/**/*.go" | ||
| - "**/cli/**/*.go" | ||
| - "**/cmd/**/*.go" | ||
| - "**/codec/**/*.go" | ||
| - "**/consts/**/*.go" | ||
| - "**/context/**/*.go" | ||
| - "**/crypto/**/*.go" | ||
| - "**/event/**/*.go" | ||
| - "**/examples/**/*.go" | ||
| - "**/extension/**/*.go" | ||
| - "**/fees/**/*.go" | ||
| - "**/genesis/**/*.go" | ||
| - "**/internal/**/*.go" | ||
| - "**/keys/**/*.go" | ||
| - "**/proto/**/*.go" | ||
| - "**/pubsub/**/*.go" | ||
| - "**/requester/**/*.go" | ||
| - "**/snow/**/*.go" | ||
| - "**/state/**/*.go" | ||
| - "**/statesync/**/*.go" | ||
| - "**/storage/**/*.go" | ||
| - "**/tests/**/*.go" | ||
| - "**/throughput/**/*.go" | ||
| - "**/utils/**/*.go" | ||
| - "**/vm/**/*.go" | ||
| - "**/x/**/*.go" | ||
| - "TEST" |
There was a problem hiding this comment.
Normally, a regex that includes all packages except chain would've been better here. However, since we plan on applying the exported rule to all public-facing packages, it might be better to leave it in this format.
export lint ruleexported lint rule
Elvis339
left a comment
There was a problem hiding this comment.
I'm not sure why isn't exported rule picking up on this... Will play a bit and see what I find out.
| Len(context.Context) int // items | ||
| Size(context.Context) int // bytes | ||
| // Number of items | ||
| Len(context.Context) int |
There was a problem hiding this comment.
Linter doesn't seem to apply exported rule correctly.
The Go documentation guidelines specify that documentation comments for exported identifiers should start with the name being declared. This helps with generated documentation, maintains a consistent style, and makes it clear what's being described, especially when multiple declarations are being documented in sequence.
// This lacks a comment
func HandleRequest(w http.ResponseWriter, r *http.Request) {
// ...
}
// Processes incoming HTTP requests
func HandleRequest(w http.ResponseWriter, r *http.Request) {
// ...
}Both of these functions should be targeted.
GoLand IDE is reporting invalid comment pattern as well.
|
Putting this back into draft; realized that |
Co-authored-by: Elvis <43846394+Elvis339@users.noreply.github.com> Signed-off-by: rodrigo <77309055+RodrigoVillar@users.noreply.github.com>
| exclude: | ||
| - 'Error return value of | ||
| .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). | ||
| is not checked' | ||
| - '(comment on exported (method|function|type|const)|should have( a | ||
| package)? comment|comment should be of the form)' | ||
| - 'func name will be used as test\.Test.* by other packages, and that | ||
| stutters; consider calling this' | ||
| - '(possible misuse of unsafe.Pointer|should have signature)' | ||
| - 'SA4011' | ||
| - 'G103: Use of unsafe calls should be audited' | ||
| - 'G204: Subprocess launched with variable' | ||
| - 'G104' | ||
| - '(G301|G302|G307): Expect (directory permissions to be 0750|file | ||
| permissions to be 0600) or less' | ||
| - 'G304: Potential file inclusion via variable' | ||
| - '(ST1000|ST1020|ST1021|ST1022)' |
There was a problem hiding this comment.
@Elvis339 these are the list of regex exclusions that were previously enforced by exclude-use-defaults, but now have to be manually excluded.
To see the full list of default regex exclusions (including the revive ones we're now enforcing), you can query via golangci-lint run --help and scroll to the --exclude-use-default flag
|
Closing this in favor in a future PR which targets a smaller package. The goal of this PR was to commit to eventually adding documentation for all PRs (via A future PR should target the |

This PR applies the
exportedlint rule to thechainpackage, which forces documentation on exported functions and interfaces.Long term, we will want to apply
exportedto all public facing packages, but this is a good starting point.